fix: restored notebook have invalid keybinding #4174
Conversation
Walkthrough่ฏฅๆๅ่ฏทๆฑๅฏน Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? ๐ชง TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
๐งน Outside diff range and nitpick comments (1)
packages/notebook/src/browser/libro-keybind-contribution.ts (1)
Line range hint
89-119: ไฟฎๅค้ฎ็ปๅฎไธญ็ฉบ็ๅฝไปคๅญๆฎตๅจ
registerKeybindingsๆนๆณไธญ๏ผ'f1'ใ'f8'ใ'f9' ้ฎ็ปๅฎ็commandๅญๆฎตไธบ็ฉบ๏ผๅฏ่ฝๅฏผ่ด้ฎ็ปๅฎๆ ๆณ็ๆใ่ฏทไธบ่ฟไบ้ฎ็ปๅฎๆๅฎๆญฃ็กฎ็ๅฝไปคใ่ฏทๅบ็จไปฅไธไฟฎๆน๏ผ
{ keybinding: 'f1', - command: '', + command: '้่ฆๆง่ก็ๅฝไปคID', when: 'libroNotebookFocused', }, { keybinding: 'f8', - command: '', + command: '้่ฆๆง่ก็ๅฝไปคID', when: 'libroNotebookFocused', }, { keybinding: 'f9', - command: '', + command: '้่ฆๆง่ก็ๅฝไปคID', when: 'libroNotebookFocused', },่ฏทๅฐ
'้่ฆๆง่ก็ๅฝไปคID'ๆฟๆขไธบๅฎ้ ็ๅฝไปคๆ ่ฏ็ฌฆใ
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
๐ Files selected for processing (2)
packages/notebook/src/browser/libro-keybind-contribution.ts(2 hunks)packages/notebook/src/browser/libro/libro-opensumi-content-contribution.ts(2 hunks)
๐ Additional comments (7)
packages/notebook/src/browser/libro/libro-opensumi-content-contribution.ts (2)
16-16: ่กจ่พพๅผๆ ผๅผไผๅ
ๆฌๅท็ๆทปๅ ๆ้ซไบๆกไปถ่กจ่พพๅผ็ๅฏ่ฏปๆง๏ผ่ฟๆฏไธไธชๅฅฝ็ๆน่ฟใ
36-37: ้ช่ฏๆไปถๅๅค็็ๆนๅจๅฝฑๅ
ๅฐ uri.path.name ๆนไธบ uri.path.base ๆฏไธไธช้่ฆๅๆด๏ผ้่ฆ็กฎไฟๅจๆๆๅบๆฏไธ้ฝ่ฝๆญฃ็กฎๅค็ๆไปถๅใๅฆๅค๏ผTODO ๆณจ้ๆๅฐ็็ฌ่ฎฐๆฌๆ น็ฎๅฝ้ฎ้ข้่ฆ่ฟไธๆญฅ่ฎจ่ฎบใ
่ฟ่กไปฅไธ่ๆฌๆฅ้ช่ฏ็ธๅ ณไปฃ็ ๏ผ
โ Verification successful
Let me gather more information about the URI path handling in the codebase.
ๆไปถๅๅค็ๆนๅจๅทฒๅพๅฐ้ช่ฏ
ไปฃ็ ๅบไธญๅทฒๅนฟๆณไฝฟ็จ uri.path.base ไฝไธบ่ทๅๆไปถๅ็ๆ ๅๆนๅผ๏ผๅ
ๆฌ็ผ่พๅจใๆไปถๆ ใ่ฐ่ฏ็ญๅคไธชๆ ธๅฟๆจกๅใๅฐ uri.path.name ๆนไธบ uri.path.base ๆฏ็ฌฆๅ้กน็ฎ่ง่็ๆญฃ็กฎๆนๅจใ
ๅ ณไบ็ฌ่ฎฐๆฌๆ น็ฎๅฝ็ TODO ๆณจ้๏ผ
- ่ฟๆฏไธไธชๅทฒ็ฅ็ๅฑ้ๆง๏ผ็ฑไบ Jupyter ๆชๆไพ่ทๅๆ น็ฎๅฝ็ API
- ็ฎๅ็ๅฎ็ฐๆฏไธไธชๅ็็ไธดๆถๆนๆก
- ๅปบ่ฎฎๅจ Jupyter ๆไพ็ธๅ ณ API ๅๅไผๅๆญคๅค็้ป่พ
๐ Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# ๆ่ฟฐ๏ผๆฃๆฅไปฃ็ ๅบไธญๆฏๅฆๅญๅจๅ
ถไปไฝฟ็จ uri.path.name ็ๅฐๆน๏ผไปฅๅ็ฌ่ฎฐๆฌๆ น็ฎๅฝ็็ธๅ
ณๅฎ็ฐ
# ๆ็ดข uri.path.name ็ไฝฟ็จ
rg "uri\.path\.name"
# ๆ็ดข็ฌ่ฎฐๆฌๆ น็ฎๅฝ็ธๅ
ณ็ๅฎ็ฐ
rg -i "notebook.*root.*dir|notebook.*root.*path"
Length of output: 508
Script:
#!/bin/bash
# Check the implementation and usage of uri.path.base
rg "uri\.path\.base"
# Check the context of workspace-variable-contribution.ts where uri.path.name is used
rg -A 5 -B 5 "uri\.path\.name" packages/workspace/src/browser/workspace-variable-contribution.ts
# Check the context of resource.ts where uri.path.name is used
rg -A 5 -B 5 "uri\.path\.name" packages/core-browser/src/contextkey/resource.ts
Length of output: 2292
packages/notebook/src/browser/libro-keybind-contribution.ts (5)
1-1: ๅฏผๅ
ฅๆจกๅๆญฃ็กฎ
ๅฟ ่ฆ็ๆจกๅๅทฒๆญฃ็กฎๅฏผๅ ฅ๏ผ็กฎไฟไบๅ่ฝ็ๆญฃๅธธไฝฟ็จใ
7-14: ๆฐๅขๆฅๅฃๅฏผๅ
ฅ็กฎไฟๅ่ฝๆฉๅฑ
ๅฏผๅ
ฅไบ CommandContributionใKeybindingContribution ็ญๆฅๅฃ๏ผๆปก่ถณไบๅ็ปญๅฎ็ฐ็ธๅบๅ่ฝ็้ๆฑใ
21-22: ็ฑปๅฃฐๆๆดๆฐ็ฌฆๅ่ฎพ่ฎก่ฆๆฑ
LibroKeybindContribution ็ฑป็ฐๅจๅฎ็ฐไบ ClientAppContributionใKeybindingContribution ๅ CommandContribution ๆฅๅฃ๏ผ็กฎไฟ็ฑปๅ
ทๅค็ธๅบ็ๅ่ฝ๏ผๅฎ็ฐไบๆ้็ๅฅ็บฆใ
36-36: ๆๅๅๅปบไธไธๆ้ฎ
ๅๅปบไบไธไธๆ้ฎ libroNotebookFocused๏ผๅๅงๅผ่ฎพไธบ false๏ผๆๅฉไบๅจไธๅ็ปไปถ้ดๅ
ฑไบซ็ถๆใ
31-33: ็กฎ่ฎค็ๅฝๅจๆๆนๆณ็ๆญฃ็กฎๆฟๆข
onStart() ๆนๆณ่ขซๆฟๆขไธบ initialize()ใ่ฏท็กฎไฟๆญคๆดๆน็ฌฆๅๆกๆถๅฏน็ๅฝๅจๆๆนๆณ็ๆๆ๏ผไปฅ้ฟๅ
ๅๅงๅ้ฎ้ขใ
ไธบ็กฎ่ฎคๆกๆถ้ขๆ็็ๅฝๅจๆๆนๆณ๏ผๅปบ่ฎฎ่ฟ่กไปฅไธ่ๆฌ๏ผ
โ Verification successful
Let me gather more information about the lifecycle methods in the codebase.
็ๅฝๅจๆๆนๆณๆฟๆขๆฏๆญฃ็กฎ็
ๆ นๆฎ ClientAppContribution ๆฅๅฃ็ๅฎไน๏ผinitialize() ๆฏๆกๆถ้ขๆ็ๅฏ้็ๅฝๅจๆๆนๆณ๏ผ็จไบๅจๅบ็จๅฏๅจๆถใๅจๅฝไปคใๆ้ฎ็ปๅฎๅ่ๅๅๅงๅไนๅๆง่กใ่ฟไธชๆฟๆขๆฏ็ฌฆๅๆกๆถ่ฎพ่ฎก็ใ
๐ Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# ๆฃๆฅๅฎ็ฐไบ ClientAppContribution ๆฅๅฃ็็ฑป๏ผไฝฟ็จไบๅชไบ็ๅฝๅจๆๆนๆณ
rg 'class .* implements .*ClientAppContribution' -A 5 | rg 'initialize\(\)|onStart\(\)'
Length of output: 455
Script:
#!/bin/bash
# ๆฃๆฅ ClientAppContribution ๆฅๅฃ็ๅฎไน
rg "interface.*ClientAppContribution" -A 10
# ๆฃๆฅๆดๅคไฝฟ็จ initialize() ็็คบไพ
ast-grep --pattern 'initialize() {
$$$
}'
# ๆฃๆฅๆดๅคไฝฟ็จ onStart() ็็คบไพ
ast-grep --pattern 'onStart() {
$$$
}'
Length of output: 1116
| registerCommands(commands: CommandRegistry) { | ||
| commands.registerCommand(NotebookCommands['EnterCommandMode'], { | ||
| execute: () => { | ||
| this.libroCommandRegistry.executeCommand(NotebookCommands['EnterCommandMode'].id); | ||
| }, | ||
| }); | ||
| this.commandRegistry.registerCommand(NotebookCommands['RunCell'], { | ||
| commands.registerCommand(NotebookCommands['RunCell'], { | ||
| execute: () => { | ||
| this.libroCommandRegistry.executeCommand(NotebookCommands['RunCell'].id); | ||
| }, | ||
| }); | ||
| this.commandRegistry.registerCommand(NotebookCommands['RunCellAndSelectNext'], { | ||
| commands.registerCommand(NotebookCommands['RunCellAndSelectNext'], { | ||
| execute: () => { | ||
| this.libroCommandRegistry.executeCommand(NotebookCommands['RunCellAndSelectNext'].id); | ||
| }, | ||
| }); | ||
| this.commandRegistry.registerCommand(NotebookCommands['RunCellAndInsertBelow'], { | ||
| commands.registerCommand(NotebookCommands['RunCellAndInsertBelow'], { | ||
| execute: () => { | ||
| this.libroCommandRegistry.executeCommand(NotebookCommands['RunCellAndInsertBelow'].id); | ||
| }, | ||
| }); | ||
| this.commandRegistry.registerCommand(NotebookCommands['SplitCellAntCursor'], { | ||
| commands.registerCommand(NotebookCommands['SplitCellAntCursor'], { | ||
| execute: () => { | ||
| this.libroCommandRegistry.executeCommand(NotebookCommands['SplitCellAntCursor'].id); | ||
| }, | ||
| }); | ||
| this.commandRegistry.registerCommand(LibroSearchToggleCommand.ShowLibroSearch, { | ||
| commands.registerCommand(LibroSearchToggleCommand.ShowLibroSearch, { |
There was a problem hiding this comment.
๐ ๏ธ Refactor suggestion
ๅปบ่ฎฎไผๅๅฝไปคๆณจๅไปฅๅๅฐ้ๅคไปฃ็
registerCommands ๆนๆณไธญๅคๆฌก้ๅคๆณจๅๅฝไปค๏ผๅปบ่ฎฎๆๅๅ
ฌๅ
ฑ้ป่พ๏ผไฝฟ็จๅพช็ฏๆๆ ๅฐๆฅ็ฎๅไปฃ็ ๏ผๆๅๅฏ่ฏปๆงๅๅฏ็ปดๆคๆงใ
ๅฏไปฅ่่ๅฆไธไฟฎๆน๏ผ
registerCommands(commands: CommandRegistry) {
const notebookCommands = [
NotebookCommands['EnterCommandMode'],
NotebookCommands['RunCell'],
NotebookCommands['RunCellAndSelectNext'],
NotebookCommands['RunCellAndInsertBelow'],
NotebookCommands['SplitCellAntCursor'],
LibroSearchToggleCommand.ShowLibroSearch,
];
notebookCommands.forEach((command) => {
commands.registerCommand(command, {
execute: () => {
this.libroCommandRegistry.executeCommand(command.id);
},
});
});
}
There was a problem hiding this comment.
ๅ ถๅฎ่ฟไธชๅปบ่ฎฎๆบๅฅฝ็ cc @zhanba
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Codecov ReportAll modified and coverable lines are covered by tests โ
Additional details and impacted files@@ Coverage Diff @@
## main #4174 +/- ##
==========================================
- Coverage 54.06% 54.06% -0.01%
==========================================
Files 1612 1612
Lines 97917 97917
Branches 20051 20051
==========================================
- Hits 52941 52940 -1
- Misses 37365 37366 +1
Partials 7611 7611
Flags with carried forward coverage won't be shown. Click here to find out more. โ View full report in Codecov by Sentry. ๐จ Try these New Features:
|
Types
Background or solution
Changelog
Summary by CodeRabbit
ๆฐๅ่ฝ
ไฟฎๅค